Only connect SecondaryRecord to secondary DB when MULTI_DB_TEST is set#318
Open
OdenTakashi wants to merge 1 commit intomainfrom
Open
Only connect SecondaryRecord to secondary DB when MULTI_DB_TEST is set#318OdenTakashi wants to merge 1 commit intomainfrom
OdenTakashi wants to merge 1 commit intomainfrom
Conversation
## Problem
`database.yml` defines the `secondary` database configuration only when `MULTI_DB_TEST=true`.
However, `SecondaryRecord` always declares:
```rb
connects_to database: { writing: :secondary }
```
As a result, running `annotaterb models` without `MULTI_DB_TEST` set raised the following error:
```sh
Unable to process app/models/secondary/test_default.rb: The secondary database is not
configured for the development environment.
```
## Solution
Make `SecondaryRecord` connect to the `secondary` database only when `MULTI_DB_TEST=true` is set.
When `MULTI_DB_TEST` is not set, `SecondaryRecord` falls back to the primary connection. Since it is an abstract class and its child class `Secondary::TestDefault` does not have a corresponding table in the primary database, both are safely skipped by annotaterb.
OdenTakashi
force-pushed
the
fix-secondary-record-adapter-not-specified
branch
from
b41557b to
b2b1458
Compare
Owner
|
This makes sense to me, but are there tests that test this change? I would think that this would break some tests -- but it could be we don't have them to test the secondary database. |
drwl
approved these changes
Mar 5, 2026
Collaborator
Author
|
Thank you for reviewing.
The existing multi-database tests all run with MULTI_DB_TEST=true, so they continue to pass as before. The single-database integration tests already cover the case without it.
We do have them in annotate_models_in_multi_db_spec.rb — they just always set MULTI_DB_TEST=true, so this change doesn't affect them. Let me know if I'm misunderstanding anything! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
database.ymldefines thesecondarydatabase configuration only whenMULTI_DB_TEST=true.However,
SecondaryRecordalways declares:As a result, running
annotaterb modelswithoutMULTI_DB_TESTset raised the following error:Unable to process app/models/secondary/test_default.rb: The secondary database is not configured for the development environment.Solution
Make
SecondaryRecordconnect to thesecondarydatabase only whenMULTI_DB_TEST=trueis set.When
MULTI_DB_TESTis not set,SecondaryRecordfalls back to the primary connection. Since it is an abstract class and its child classSecondary::TestDefaultdoes not have a corresponding table in the primary database, both are safely skipped by annotaterb.This change only affects the test dummy app (
spec/dummyapp/) and does not impact the gem’s runtime behavior.